asset: WasmAssetIo #703
asset: WasmAssetIo #703
Conversation
There was a problem hiding this comment.
Nice work!
Regarding block_on, I think it would be best to make loaders async as it is the only way to make it work on single threaded wasm. GLTF loading with sub assets would still be broken, but that can be fixed by making schedule async as in the wasm threads PR.
| impl AssetIo for WasmAssetIo { | ||
| async fn load_path(&self, path: &Path) -> Result<Vec<u8>, AssetIoError> { | ||
| let path = self.root_path.join(path); | ||
| let window = web_sys::window().unwrap(); |
There was a problem hiding this comment.
This will not work in web workers and WorkerGlobalScope should be used instead. However, web-sys doesn't seem to provide any abstraction to access this cross-environments as WindowOrWorkerGlobalScope mixin is not supported.
I think the best option would be to access js_sys::global() and try casting it to either Window or WorkerGlobalScope, whichever succeeds. Similar fix was done in rustwasm/gloo#106, but it's kind of ugly. Maybe it could land in bevy_utils?
There was a problem hiding this comment.
haha anything specific about web-sys / webworkers is going to be out of my depth at the moment. but that sounds reasonable to me. given that we know this works outside of webworkers, im inclined to merge as-is for now. Then someone who knows what they're doing (like you) can follow up with webworker compat.
There was a problem hiding this comment.
Regarding
block_on, I think it would be best to make loaders async as it is the only way to make it work on single threaded wasm. GLTF loading with sub assets would still be broken, but that can be fixed by making schedule async as in the wasm threads PR.
+1 for making loaders async - I did it on my branch and it was very simple change
There was a problem hiding this comment.
I was hesitant to make asset loaders async because I didn't want consumers to need the #[async_trait] proc-macro. But:
- atelier-assets uses async loaders
- they do it without requiring
#[async_trait]in consumers by using BoxFuture - even if async_trait was required, thats not a huge loss relative to what we get
I'll make the change
There was a problem hiding this comment.
here is my implementation, for example working gltf loader: mrk-its@05bf8ae
BTW I didn't know how to conditionally enable '?Send' without copying whole struct definition, that's why I've ended with wrapping web_sys types with unsafe impl Send :) good to know about #[cfg_attr] :)
|
Not a huge fan of the reduced clarity of: impl AssetLoader for HdrTextureLoader {
fn load<'a>(&'a self, bytes: &'a [u8], load_context: &'a mut LoadContext) -> BoxedFuture<'a, Result<()>> {
Box::pin(async move {
Ok(())
})
}
fn extensions(&self) -> &[&str] {
static EXTENSIONS: &[&str] = &["hdr"];
EXTENSIONS
}
}instead of #[async_trait]
impl AssetLoader for HdrTextureLoader {
fn load(&self, bytes: &[u8], load_context: &mut LoadContext) -> Result<()> {
Ok(())
}
fn extensions(&self) -> &[&str] {
static EXTENSIONS: &[&str] = &["hdr"];
EXTENSIONS
}
}But I think I still prefer it over requiring consumers to pull in async_trait |
|
I totally get that first class async traits are hard. But dang thats a huge missing piece. It sounds like the "real" fix is a long way off (GATs and a few other features are required). Imo it makes sense to bake in |
karroffel
added
A-Assets
This adds a partial AssetIo wasm implementation. Loading folders doesn't work (just returns an empty list for now) and loading files within AssetLoaders will probably break (because we use futures::block_on). Some GLTF files won't load as a result of this. But the asset example works!
The latest cursor grabbing api broke wasm winit setup, so I fixed that too.
@mrk-its @chemicstry